Fix webview after changes in upstream. #563
Conversation
Co-authored-by: Oleksandr Andriienko <[email protected]> Signed-off-by: Mykola Morhun <[email protected]>
| @@ -76,7 +76,8 @@ if [ "${NOCDN}" == "true" ]; then | |||
| fi | |||
| shopt -u nocasematch | |||
|
|
|||
| # run che | |||
| # run Che Theia | |||
| export THEIA_WEBVIEW_EXTERNAL_ENDPOINT=$(node get-webview-route.js) | |||
There was a problem hiding this comment.
question: we don't have the endpoint as environment variable ?
There was a problem hiding this comment.
I mean, che-server is not providing that information to containers through env vars ?
There was a problem hiding this comment.
No, we don't have external route in env vars.
| **********************************************************************/ | ||
|
|
||
| const fs = require('fs'); | ||
| const workspaceClient = require('@eclipse-che/workspace-client'); |
There was a problem hiding this comment.
where does it find this dependency ?
There was a problem hiding this comment.
It gets the dependency from Che Theia node modules.
There was a problem hiding this comment.
In the dockerfile we copy script to the /home/theia, where is located node_modules
| @@ -163,5 +163,6 @@ COPY --from=builder /home/theia-dev/theia-source-code/production/plugins /defaul | |||
| COPY --chown=theia:root --from=builder /home/theia-dev/theia-source-code/production /home/theia | |||
| USER theia | |||
| WORKDIR /projects | |||
| COPY src/get-webview-route.js ${HOME}/get-webview-route.js | |||
There was a problem hiding this comment.
Should it be possible (maybe later) to use a runtime information instead of using environment variable.
Explanation: Here we're using a shell script before running the theia process in the container (by setting environment variable)
I think it would be cleaner if we bring a hook/extension point of theia at runtime by updating the setting on the fly. So no need to use separate script and fix env variable. It's when theia webview will resolve stuff that it will deal with che-theia code
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes, I think we can do it in runtime.
There was a problem hiding this comment.
I think we should try, but as for now I would like to merge current PR as it is tested and fixes (for https case) blocker issue.
There was a problem hiding this comment.
yes this is why I said later
|
❌ E2E Happy path tests failed ❗
See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
|
Here is the image of welcome plug-in (so there is not only icon issue on my side) |
Known issue eclipse-che/che#15324 |
|
@AndrienkoAleksandr I know about the icon as I said, my question was more on the font colors/style: Before: |
Yes, Ok, I will take a look what was changed. But from first look welcome plugin used some styles from theia, and am not sure that they are accessible after web-view rework. |
|
@AndrienkoAleksandr sure, we can't get exact same stuff than before. |
Ok, If @slemeur is ok with current rendering I will close this one eclipse-che/che#15334 |
Signed-off-by: Eric Williams <[email protected]>
Co-authored-by: Oleksandr Andriienko [email protected]
Signed-off-by: Mykola Morhun [email protected]
What does this PR do?
Fixes webview after changes in upstream Eclipse Theia.
According to changes in upstream Eclipse Theia, each webview is run on its own domain by default. In this PR we reconfigure Theia to run all webviews on the same as Che Theia host (actually as it was before).
Also after upstream changes it is possible to run webviews only with secure connection (requires https protocol), the only exception is localhost for testing purposes.
What issues does this PR fix or reference?
eclipse-che/che#15283